Scope OG rendering assets to each request - #1087
Merged
Merged
Conversation
📝 WalkthroughWalkthroughOG asset loading now uses keyed image assets with separate font and image caches. Standard OG and README header generators preload the required assets and pass them to ChangesOG asset loading and rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant StandardOG as Standard OG generator
participant READMEHeader as README header generator
participant loadOgAssets
participant renderOgImage
StandardOG->>loadOgAssets: load logo asset and fonts
loadOgAssets-->>StandardOG: return fonts and logo image data
StandardOG->>renderOgImage: render with preloaded assets
READMEHeader->>loadOgAssets: load theme emblem and fonts
loadOgAssets-->>READMEHeader: return fonts and theme emblem data
READMEHeader->>renderOgImage: render with preloaded assets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | 3b83397 | Commit Preview URL Branch Preview URL |
Jul 31 2026, 09:15 PM |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/server/og/assets.server.ts (1)
68-97: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse single-flight caches for cold requests.
cachedFontsandcachedImagesare populated only after loading completes. Concurrent cold requests can each start duplicate font and image loads. Cache the pendingPromisevalues, and remove a rejected promise so later requests can retry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/og/assets.server.ts` around lines 68 - 97, Update loadFonts and loadOgAssets to cache in-flight Promise values rather than only completed results, so concurrent cold requests share one font or image load. Store the pending promise before awaiting it, and remove the corresponding cachedFonts or cachedImages entry when loading rejects so subsequent requests can retry; preserve the existing resolved return values and asset mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/server/og/assets.server.ts`:
- Around line 68-97: Update loadFonts and loadOgAssets to cache in-flight
Promise values rather than only completed results, so concurrent cold requests
share one font or image load. Store the pending promise before awaiting it, and
remove the corresponding cachedFonts or cachedImages entry when loading rejects
so subsequent requests can retry; preserve the existing resolved return values
and asset mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 263d6b8c-b421-45de-a29d-cf063e233ca4
📒 Files selected for processing (2)
src/server/og/assets.server.tssrc/server/og/generate.server.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Evidence\n\nPR #1076 expanded the shared cold render path from three static assets to five and registered the logo plus both emblems on every request, although each rendered image uses exactly one of them.\n\n## Impact\n\n- Standard /api/og requests stop fetching and registering two unused emblems: two fewer asset-binding reads and 17,685 fewer bytes.\n- README header requests load only the emblem for the requested theme, avoiding the unused logo and opposite-theme emblem: one fewer registration and roughly 18 KB less asset data.\n- Fonts and the selected image load concurrently and remain cached independently.\n\nThe generated image contract and cache headers are unchanged.\n\n## Validation\n\n- pnpm test: TypeScript, type-aware lint, and 140 tests (139 passed, 1 skipped)\n- Full scripts/og-preview.ts render set\n- Full pnpm run readme:preview render set\n- git diff --check\n\n## Risk\n\nLow. The change only scopes the existing asset loader and ImageResponse registrations; both render paths completed their full preview matrices.
Summary by CodeRabbit
Improvements
Bug Fixes